• pnpm-lock.yaml: Language not supported

backend/src/migrations/1777039182865-AddDynamoDbSchemaChangeTypes.ts-19-29 (1)

19-29: ⚠️ Potential issue | 🟡 Minor

The down() migration will fail if any rows have DynamoDB change types.

When reverting, if any table_schema_change rows have changeType values like DYNAMODB_CREATE_TABLE, the cast on Line 24 will fail since those values don't exist in the recreated old enum. Consider either:

1. Documenting that this migration is non-revertible if DynamoDB changes exist, or
2. Adding a pre-check or data cleanup step before the enum revert.

This is acceptable if DynamoDB schema changes are not yet in production, but could cause issues during development rollbacks.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@backend/src/migrations/1777039182865-AddDynamoDbSchemaChangeTypes.ts` around
lines 19 - 29, The down() migration can fail when reverting the enum because
rows in table_schema_change may contain DynamoDB values (e.g.,
DYNAMODB_CREATE_TABLE) that are not present in the recreated
"table_schema_change_changetype_enum_old"; before casting in down() (the ALTER
TABLE ... USING ... on column changeType) add a safe cleanup or guard: either
delete or map/convert any rows whose changeType is a DynamoDB-specific value to
a supported fallback (or NULL) or bail with a clear error documenting
non-revertible state; implement this check/cleanup in the migration before the
ALTER TABLE or change the migration to be non-revertible by throwing/returning
early from down() with an explanatory message. Ensure you modify the down()
method in the migration that manipulates the "table_schema_change" table and the
"table_schema_change_changetype_enum_old"/"table_schema_change_changetype_enum"
enum names.

backend/src/migrations/1776857063726-AddTableSchemaChangeEntity.ts-63-72 (1)

63-72: ⚠️ Potential issue | 🟡 Minor

The down() migration will fail if any rows use the new comparator values.

If any table_filters rows have dynamic_filter_comparator set to 'in' or 'between', the enum recast on Line 67 will fail. This is the same pattern as the DynamoDB migration issue.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@backend/src/migrations/1776857063726-AddTableSchemaChangeEntity.ts` around
lines 63 - 72, The down() migration in AddTableSchemaChangeEntity will fail if
any table_filters.dynamic_filter_comparator rows contain the new values ('in' or
'between'); before changing the column type you must normalize those rows. In
the down() method, prior to creating
table_filters_dynamic_filter_comparator_enum_old and running the ALTER COLUMN,
add a queryRunner.query(...) UPDATE that replaces new enum values with valid old
ones (e.g. UPDATE "table_filters" SET "dynamic_filter_comparator" = 'contains'
WHERE "dynamic_filter_comparator" IN ('in','between');), then proceed with
creating the old enum, ALTER COLUMN, DROP the new enum and RENAME the old enum
as currently implemented. Ensure you reference the same column name
("dynamic_filter_comparator") and enum names
("table_filters_dynamic_filter_comparator_enum_old",
"table_filters_dynamic_filter_comparator_enum") when adding the UPDATE.

backend/package.json-82-82 (1)

82-82: ⚠️ Potential issue | 🟡 Minor

Consider updating node-sql-parser to version 5.4.0.

The node-sql-parser dependency is appropriate for DDL validation. However, version 5.4.0 was released on January 12, 2026. Update from ^5.3.0 to ^5.4.0 to use the latest stable version.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@backend/package.json` at line 82, Update the node-sql-parser dependency in
package.json from "^5.3.0" to "^5.4.0" (change the value for the
"node-sql-parser" entry), then run your package manager (npm install or yarn
install) to refresh node_modules and update the lockfile (package-lock.json or
yarn.lock); after upgrading, run the test/build commands to ensure DDL
validation still passes and commit the updated lockfile with the package.json
change.

backend/src/entities/table-schema/utils/schema-change-ownership.guard.ts-26-29 (1)

26-29: ⚠️ Potential issue | 🟡 Minor

Add defensive check for request.decoded.

If request.decoded is undefined (e.g., middleware not run), accessing .sub will throw an unhandled error. Consider adding validation or relying on middleware ordering guarantees.

🛡️ Proposed defensive check

 	async canActivate(context: ExecutionContext): Promise<boolean> {
 		const request: IRequestWithCognitoInfo = context.switchToHttp().getRequest();
+		if (!request.decoded?.sub) {
+			throw new BadRequestException('Authentication required.');
+		}
 		const userId = request.decoded.sub;
 		const changeId: string = request.params?.changeId;

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@backend/src/entities/table-schema/utils/schema-change-ownership.guard.ts`
around lines 26 - 29, The canActivate method currently reads request.decoded.sub
without checking request.decoded; add a defensive check in canActivate (in file
containing function canActivate and type IRequestWithCognitoInfo) to verify
request?.decoded and request.decoded.sub exist before using them, and if missing
either throw an UnauthorizedException (preferred) or return false so the guard
fails safely; ensure changeId handling remains unchanged after the check.

backend/src/entities/table-schema/utils/clickhouse-ddl.ts-14-20 (1)

14-20: ⚠️ Potential issue | 🟡 Minor

Empty host fallback produces invalid URL.

When connection.host is null or undefined, it defaults to an empty string, resulting in a malformed URL like http://:8123. This will cause a runtime error when createClient attempts to connect.

Consider throwing a descriptive error for missing required fields:

🛡️ Proposed fix: Validate required host

 export async function executeClickHouseDdl(connection: ClickHouseExecutionConnection, sql: string): Promise<void> {
-	const host = connection.host ?? '';
+	const host = connection.host;
+	if (!host) {
+		throw new Error('ClickHouse connection requires a host');
+	}
 	const port = connection.port ?? 8123;
 	const protocol = connection.ssl ? 'https' : 'http';

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@backend/src/entities/table-schema/utils/clickhouse-ddl.ts` around lines 14 -
20, The URL construction in executeClickHouseDdl uses connection.host ?? ''
which yields malformed URLs like "http://:8123" when host is missing; update the
function to validate that connection.host is present and non-empty (and
optionally that connection.port is a valid number) before building
NodeClickHouseClientConfigOptions, and throw a descriptive error (e.g., "Missing
ClickHouse host in connection") instead of proceeding to createClient so the
caller receives a clear failure instead of a runtime URL/connection error.

backend/src/migrations/1777295364595-AddBatchColumnsToTableSchemaChange.ts-12-15 (1)

12-15: ⚠️ Potential issue | 🟡 Minor

Inconsistent schema qualification in index creation and deletion.

The up migration creates the index without schema prefix (CREATE INDEX "IDX_tsc_batch_order"), but the down migration drops it with explicit "public" schema (DROP INDEX "public"."IDX_tsc_batch_order"). This inconsistency violates the codebase pattern where other migrations consistently qualify indexes with the schema. Add the "public" schema prefix to the CREATE INDEX statement in the up migration to match the pattern and ensure consistency.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@backend/src/migrations/1777295364595-AddBatchColumnsToTableSchemaChange.ts`
around lines 12 - 15, The CREATE INDEX in the up method is missing the "public"
schema prefix while down drops "public"."IDX_tsc_batch_order"; update the up
migration (method up in AddBatchColumnsToTableSchemaChange / function creating
the index) to use CREATE INDEX "public"."IDX_tsc_batch_order" so it matches the
DROP INDEX in down and follows the repository's schema qualification convention.

backend/test/ava-tests/non-saas-tests/non-saas-table-schema-elasticsearch-e2e.test.ts-300-346 (1)

300-346: ⚠️ Potential issue | 🟡 Minor

This test name overstates what it verifies.

The case says “rollback is recorded”, but it never calls /rollback and never asserts the rollback audit row. That leaves the Elasticsearch updateMapping rollback behavior uncovered. Either rename the test to match the current assertions, or add the rollback call and audit checks.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In
`@backend/test/ava-tests/non-saas-tests/non-saas-table-schema-elasticsearch-e2e.test.ts`
around lines 300 - 346, The test named "Elasticsearch: updateMapping adds a new
field; rollback is recorded" does not actually exercise or assert rollback;
either rename it to reflect only the approval/update behavior or add the missing
rollback flow: after approving (function/test block around changeId and
approveResp) POST to /table-schema/change/{changeId}/rollback with the same
Cookie, assert a successful response, then query the audit/changes endpoint or
DB to assert a rollback audit row exists and that the recorded rollbackOp
matches the expected JSON (referencing nextProposal.rollbackOp and
SchemaChangeTypeEnum.ELASTICSEARCH_UPDATE_MAPPING); ensure you also keep the
final mapping assertion (getMapping(indexName)) as needed.

backend/src/entities/table-schema/use-cases/reject-batch-schema-changes.use-case.ts-38-40 (1)

38-40: ⚠️ Potential issue | 🟡 Minor

Batch status updates are not wrapped in a transaction.

The loop updates each item's status individually. If an error occurs (e.g., database connectivity issue, constraint violation) after some items are updated, the batch will be left in an inconsistent state with some items rejected and others unchanged.

Consider wrapping the updates in a transaction to ensure atomicity.

💡 Suggested approach

If the repository supports transactions, wrap the updates:

await this._dbContext.connection.transaction(async (manager) => {
    for (const item of rejectable) {
        await this._dbContext.tableSchemaChangeRepository.updateStatus(
            item.id, 
            SchemaChangeStatusEnum.REJECTED,
            manager
        );
    }
});

Alternatively, add a bulkUpdateStatus method to the repository.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In
`@backend/src/entities/table-schema/use-cases/reject-batch-schema-changes.use-case.ts`
around lines 38 - 40, The loop in reject-batch-schema-changes.use-case.ts
updates each item one-by-one using
this._dbContext.tableSchemaChangeRepository.updateStatus and
SchemaChangeStatusEnum.REJECTED, leaving the batch non-atomic if an error
occurs; fix it by performing the updates inside a database transaction (e.g.,
use this._dbContext.connection.transaction and pass the transactional manager
into updateStatus) or implement and call a repository-level bulkUpdateStatus
that performs the status changes atomically. Ensure updateStatus accepts an
optional transactional manager (or create bulkUpdateStatus) and wrap the
for-loop so all rejections are committed or rolled back together.

backend/src/migrations/1776953988756-AddMongoSchemaChangeTypes.ts-19-30 (1)

19-30: ⚠️ Potential issue | 🟡 Minor

The down migration may fail if MongoDB change types exist in the database.

The rollback casts changeType to the old enum which excludes MONGO_* values. If any rows contain MongoDB-specific change types, the ALTER TABLE at line 23-25 will fail with a cast error.

Consider either:

1. Documenting this limitation in a comment.
2. Adding a pre-check or UPDATE to handle existing MongoDB rows (e.g., setting them to OTHER).

📝 Proposed documentation comment

 	public async down(queryRunner: QueryRunner): Promise<void> {
+		// WARNING: This rollback will fail if any table_schema_change rows have
+		// MONGO_* changeType values. Manually update or delete such rows first.
 		await queryRunner.query(
 			`CREATE TYPE "public"."table_schema_change_changetype_enum_old" AS ENUM('CREATE_TABLE', 'DROP_TABLE', 'ADD_COLUMN', 'DROP_COLUMN', 'ALTER_COLUMN', 'ADD_INDEX', 'DROP_INDEX', 'ADD_FOREIGN_KEY', 'DROP_FOREIGN_KEY', 'ADD_PRIMARY_KEY', 'DROP_PRIMARY_KEY', 'ROLLBACK', 'OTHER')`,
 		);

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@backend/src/migrations/1776953988756-AddMongoSchemaChangeTypes.ts` around
lines 19 - 30, The down migration in AddMongoSchemaChangeTypes.ts (public async
down(queryRunner: QueryRunner)) will fail if rows contain MONGO_* enum values;
before altering the column type, update any MongoDB-specific values to a safe
value (e.g., set changeType = 'OTHER' for values starting with 'MONGO_') or add
a clear comment documenting the limitation; modify the down method to run a
pre-check/update via queryRunner.query to convert problematic changeType values
to 'OTHER' (or skip/throw with a descriptive message) before executing the ALTER
TABLE and DROP/RENAME TYPE statements.

backend/test/ava-tests/non-saas-tests/non-saas-table-schema-mysql-e2e.test.ts-184-187 (1)

184-187: ⚠️ Potential issue | 🟡 Minor

Add clearAllTestKnex() to test teardown.

The test creates Knex instances via getTestKnex() which caches connections in an LRUCache. These cached connections are never explicitly destroyed because clearAllTestKnex() is not called in the test.after hook. While caching prevents connection pool exhaustion from repeated calls within tests, this omission leaves database connections unclosed at test completion. Call clearAllTestKnex() in the existing test.after hook to properly clean up all cached Knex instances.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In
`@backend/test/ava-tests/non-saas-tests/non-saas-table-schema-mysql-e2e.test.ts`
around lines 184 - 187, The test opens cached Knex connections via getTestKnex
but never clears them; update the existing test.after teardown to call
clearAllTestKnex() so all cached Knex instances are destroyed and DB pools
closed; locate the test.after hook in this file and add a call to
clearAllTestKnex() (alongside any existing cleanup) to ensure proper teardown.

backend/src/entities/table-schema/utils/validate-proposed-ddl.ts-160-162 (1)

160-162: ⚠️ Potential issue | 🟡 Minor

Regex issue in normalizeIdentifier — may not correctly strip all quote styles.

The regex /^[\"[]|[`"]]$/g` has issues:

1. The character class [\"]]is malformed —]needs escaping as]` to be treated literally
2. The anchors ^ and $ combined with | mean it only matches quotes at the very start OR very end, not both in one pass
3. Identifiers like "tableName" would become tableName" after one replace

Proposed fix

 function normalizeIdentifier(name: string): string {
-	return name.replace(/^[`"[]|[`"\]]$/g, '').toLowerCase();
+	return name.replace(/^[`"\[]|[\]`"]$/g, '').toLowerCase();
 }

Or more robustly, strip matching quotes:

function normalizeIdentifier(name: string): string {
	let normalized = name;
	if ((normalized.startsWith('"') && normalized.endsWith('"')) ||
	    (normalized.startsWith('`') && normalized.endsWith('`')) ||
	    (normalized.startsWith('[') && normalized.endsWith(']'))) {
		normalized = normalized.slice(1, -1);
	}
	return normalized.toLowerCase();
}

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@backend/src/entities/table-schema/utils/validate-proposed-ddl.ts` around
lines 160 - 162, The normalizeIdentifier function uses a malformed regex
(/^[`"[]|[`"\]]$/g) that won't reliably strip quoting and can leave a trailing
quote; replace the regex approach with explicit logic in normalizeIdentifier
that checks for and removes matching surrounding quote pairs (", `, and [
])—e.g., if the string starts and ends with the same quote type (or '[' and
']'), slice off the first and last characters—and then return the result
lowercased so identifiers like `"Table"` or `[Table]` become table.

backend/src/entities/table-schema/utils/assert-dialect-supported.ts-32-34 (1)

32-34: ⚠️ Potential issue | 🟡 Minor

Inconsistency: isMongoDialect includes agent_mongodb but SUPPORTED_DIALECTS does not.

The helper returns true for agent_mongodb, but assertDialectSupported would throw for that same connection type since it's not in SUPPORTED_DIALECTS. If agent_mongodb should be supported, add it to SUPPORTED_DIALECTS. Otherwise, consider whether this helper should only check the supported subset.

Proposed fix if agent_mongodb should be supported

 const SUPPORTED_DIALECTS: ReadonlySet<ConnectionTypesEnum> = new Set([
 	ConnectionTypesEnum.postgres,
 	ConnectionTypesEnum.mysql,
 	ConnectionTypesEnum.mysql2,
 	ConnectionTypesEnum.mssql,
 	ConnectionTypesEnum.oracledb,
 	ConnectionTypesEnum.ibmdb2,
 	ConnectionTypesEnum.mongodb,
+	ConnectionTypesEnum.agent_mongodb,
 	ConnectionTypesEnum.clickhouse,
 	ConnectionTypesEnum.agent_clickhouse,
 	ConnectionTypesEnum.dynamodb,
 	ConnectionTypesEnum.cassandra,
 	ConnectionTypesEnum.agent_cassandra,
 	ConnectionTypesEnum.elasticsearch,
 ]);

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@backend/src/entities/table-schema/utils/assert-dialect-supported.ts` around
lines 32 - 34, isMongoDialect currently returns true for
ConnectionTypesEnum.agent_mongodb but SUPPORTED_DIALECTS does not include that
enum, causing assertDialectSupported to reject agent_mongodb; fix by choosing
one of two actions: (A) if agent_mongodb should be treated as supported, add
ConnectionTypesEnum.agent_mongodb to SUPPORTED_DIALECTS and update any related
tests/docs so assertDialectSupported and isMongoDialect agree, or (B) if
agent_mongodb should not be supported, remove the agent_mongodb branch from
isMongoDialect (so it only checks ConnectionTypesEnum.mongodb) and update any
callers/tests that rely on isMongoDialect returning true for agent_mongodb.
Ensure consistency between isMongoDialect, SUPPORTED_DIALECTS, and
assertDialectSupported.

backend/src/migrations/1776857063726-AddTableSchemaChangeEntity.ts (1)

6-54: Consider separating unrelated schema changes into distinct migrations.

This migration bundles two logically unrelated changes:

1. The table_filters_dynamic_filter_comparator_enum modification (adding 'in', 'between')
2. The new table_schema_change entity with its enums, table, and foreign keys

Combining unrelated changes complicates debugging and selective rollbacks. If this is generated by TypeORM's migration:generate, consider splitting manually for cleaner version history.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@backend/src/migrations/1776857063726-AddTableSchemaChangeEntity.ts` around
lines 6 - 54, The migration mixes two unrelated changes: the enum/column change
for table_filters_dynamic_filter_comparator_enum and the new table_schema_change
entity (and its enums/constraints). Split this migration into two: keep queries
that create/rename/alter table_filters_dynamic_filter_comparator_enum and the
ALTER TABLE "table_filters" ALTER COLUMN call in one migration, and move the
CREATE TYPEs for table_schema_change_databasetype_enum,
table_schema_change_changetype_enum, table_schema_change_status_enum plus the
CREATE TABLE "table_schema_change", its INDEXes and its ALTER TABLE … ADD
CONSTRAINT foreign keys into a separate migration; update each migration’s up
(and corresponding down) to only contain the relevant queries so symbols to
locate are up(queryRunner: QueryRunner),
"table_filters_dynamic_filter_comparator_enum", and "table_schema_change" (and
its FK constraints FK_ab6fb01554213543f0a39f7d98e,
FK_d4a735643602c7e2337770d368b, FK_f15e652a55b856bf9c64c012d00).

backend/src/entities/table-schema/application/data-transfer-objects/schema-change-list-response.dto.ts (1)

2-2: Avoid cross-entity coupling for pagination contract.

Line 2 imports PaginationDs from the table entity package, which couples table-schema API responses to table internals. Consider using a shared/common pagination response DTO owned outside entity-specific modules.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In
`@backend/src/entities/table-schema/application/data-transfer-objects/schema-change-list-response.dto.ts`
at line 2, The SchemaChangeListResponseDto currently imports PaginationDs from
the table entity, creating cross-entity coupling; replace that with a
shared/common pagination DTO: create or use an existing shared
PaginationResponseDto (or similarly named DTO) owned by a common/shared package,
update the import in schema-change-list-response.dto.ts to reference that shared
DTO instead of PaginationDs, and adjust the SchemaChangeListResponseDto type
references to use the new shared symbol (e.g., PaginationResponseDto) so
table-schema is decoupled from table internals.

backend/src/entities/table-schema/application/data-structures/get-schema-change.ds.ts (1)

1-4: Convert to an interface—this is a pure data transfer shape.

This class is used exclusively as a type contract (never instantiated), making it a candidate for an interface per coding guidelines: **/*.{ts,tsx} should use interfaces for object shapes.

♻️ Suggested refactor

-export class GetSchemaChangeDs {
-	changeId: string;
-	userId: string;
-}
+export interface GetSchemaChangeDs {
+	changeId: string;
+	userId: string;
+}

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In
`@backend/src/entities/table-schema/application/data-structures/get-schema-change.ds.ts`
around lines 1 - 4, Replace the class GetSchemaChangeDs with a TypeScript
interface since it is only used as a type shape; change the declaration from
"export class GetSchemaChangeDs { changeId: string; userId: string; }" to an
exported interface named GetSchemaChangeDs with the same properties, and update
any imports/usages (type-only imports if applicable) so consumers treat it as a
type (no instantiation or "new") to satisfy the codebase convention of using
interfaces for data-transfer shapes.

backend/src/decorators/slug-uuid.decorator.ts (1)

6-40: Avoid duplicate sources of truth for allowed slug/UUID params.

SlugUuidParameter and availableSlagParameters currently duplicate the same list. Consider deriving the union type from a single as const array to prevent future drift.

♻️ Proposed refactor

-export type SlugUuidParameter =
-	| 'slug'
-	| 'connectionId'
-	| 'groupId'
-	| 'userId'
-	| 'actionId'
-	| 'ruleId'
-	| 'eventId'
-	| 'apiKeyId'
-	| 'companyId'
-	| 'threadId'
-	| 'filterId'
-	| 'chatId'
-	| 'changeId'
-	| 'batchId';
+const AVAILABLE_SLUG_UUID_PARAMETERS = [
+	'slug',
+	'connectionId',
+	'groupId',
+	'userId',
+	'apiKeyId',
+	'actionId',
+	'ruleId',
+	'eventId',
+	'companyId',
+	'threadId',
+	'filterId',
+	'chatId',
+	'changeId',
+	'batchId',
+] as const;
+export type SlugUuidParameter = (typeof AVAILABLE_SLUG_UUID_PARAMETERS)[number];
@@
-		const availableSlagParameters: Array<SlugUuidParameter> = [
-			'slug',
-			'connectionId',
-			'groupId',
-			'userId',
-			'apiKeyId',
-			'actionId',
-			'ruleId',
-			'eventId',
-			'companyId',
-			'threadId',
-			'filterId',
-			'chatId',
-			'changeId',
-			'batchId',
-		];
-		if (!availableSlagParameters.includes(parameterName)) {
+		if (!AVAILABLE_SLUG_UUID_PARAMETERS.includes(parameterName)) {
 			throw new BadRequestException(Messages.UUID_INVALID);
 		}

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@backend/src/decorators/slug-uuid.decorator.ts` around lines 6 - 40, The
allowed slug/UUID values are duplicated between the SlugUuidParameter union and
availableSlagParameters; refactor by extracting a single readonly array (e.g.,
const ALLOWED_SLUG_PARAMS = ['slug','connectionId',...] as const) and derive the
type via type SlugUuidParameter = typeof ALLOWED_SLUG_PARAMS[number]; then
update the SlugUuid decorator (createParamDecorator callback, parameterName
default and the includes check) to use that single array (ALLOWED_SLUG_PARAMS)
so the list of allowed keys is defined in one place.

backend/src/entities/table-schema/utils/map-schema-change-to-response-dto.ts (1)

4-30: Convert to arrow function syntax to match project TypeScript style.

This should be a const arrow function per the repository's style preference for TypeScript files.

♻️ Proposed refactor

-export function mapSchemaChangeToResponseDto(entity: TableSchemaChangeEntity): SchemaChangeResponseDto {
-	return {
+export const mapSchemaChangeToResponseDto = (
+	entity: TableSchemaChangeEntity,
+): SchemaChangeResponseDto => {
+	return {
 		id: entity.id,
 		connectionId: entity.connectionId,
 		batchId: entity.batchId,
@@
 		appliedAt: entity.appliedAt,
 		rolledBackAt: entity.rolledBackAt,
 	};
-}
+};

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@backend/src/entities/table-schema/utils/map-schema-change-to-response-dto.ts`
around lines 4 - 30, Replace the named function mapSchemaChangeToResponseDto
with a const arrow-function export to match project TypeScript style: declare
export const mapSchemaChangeToResponseDto = (entity: TableSchemaChangeEntity):
SchemaChangeResponseDto => { ... } (or use a concise object return), preserving
all field mappings (id, connectionId, batchId, orderInBatch, authorId,
previousChangeId, forwardSql, rollbackSql, userModifiedSql, status, changeType,
targetTableName, databaseType, executionError, isReversible,
autoRollbackAttempted, autoRollbackSucceeded, userPrompt, aiSummary,
aiReasoning, createdAt, appliedAt, rolledBackAt) and the existing types
TableSchemaChangeEntity and SchemaChangeResponseDto.

backend/test/ava-tests/non-saas-tests/non-saas-table-schema-redis-e2e.test.ts (1)

67-67: Unused _testUtils variable.

The _testUtils is assigned but never used in the test. Consider removing it if not needed, or add a comment explaining its intended future use.

♻️ Remove if not needed

 const mockFactory = new MockFactory();
 let app: INestApplication;
-let _testUtils: TestUtils;

 test.before(async () => {
 	setSaasEnvVariable();
 	const moduleFixture = await Test.createTestingModule({
-		imports: [ApplicationModule, DatabaseModule],
-		providers: [DatabaseService, TestUtils],
+		imports: [ApplicationModule, DatabaseModule],
+		providers: [DatabaseService],
 	})

And remove line 79:

-	_testUtils = moduleFixture.get<TestUtils>(TestUtils);

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In
`@backend/test/ava-tests/non-saas-tests/non-saas-table-schema-redis-e2e.test.ts`
at line 67, Remove the unused variable declaration _testUtils of type TestUtils
(it is declared but never referenced); either delete the line declaring let
_testUtils: TestUtils; or, if you intend to use it later, add a brief comment
above it explaining the planned purpose to avoid linter/test warnings—also
ensure you remove the related redundant line mentioned in the review (the later
unused assignment) so no dead/unused TestUtils symbols remain (search for
_testUtils, TestUtils in the test file to remove or document consistently).

backend/src/entities/table-schema/use-cases/list-schema-changes.use-case.ts (1)

30-32: Consider validating pagination inputs earlier.

When limit <= 0, the fallback to perPage = 1 is defensive, but combined with a non-zero offset, it produces potentially confusing pagination metadata (e.g., limit=0, offset=50 yields currentPage=51, perPage=1).

Consider validating and normalizing limit/offset at the controller or DTO level, or defaulting both to sensible values here (e.g., limit = Math.max(1, limit), offset = Math.max(0, offset)).

♻️ Optional: Add input normalization

 	protected async implementation(inputData: ListSchemaChangesDs): Promise<SchemaChangeListResponseDto> {
-		const { connectionId, limit, offset } = inputData;
+		const { connectionId } = inputData;
+		const limit = Math.max(1, inputData.limit);
+		const offset = Math.max(0, inputData.offset);

 		const [items, total] = await this._dbContext.tableSchemaChangeRepository.findByConnectionPaginated(connectionId, {

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@backend/src/entities/table-schema/use-cases/list-schema-changes.use-case.ts`
around lines 30 - 32, Normalize and validate pagination inputs before computing
perPage/currentPage/lastPage: ensure limit is at least 1 and offset is
non-negative (e.g., set limit = Math.max(1, limit) and offset = Math.max(0,
offset)) inside the list-schema-changes use case (or earlier in the
controller/DTO), then compute perPage = limit, currentPage = Math.floor(offset /
perPage) + 1 and lastPage = Math.max(1, Math.ceil(total / perPage)); update
references to perPage/currentPage/lastPage in this file
(list-schema-changes.use-case.ts) so pagination metadata is always sensible.

backend/src/entities/table-schema/utils/schema-change-batch-ownership.guard.ts (1)

18-19: Move this guard into the guards layer.

This is an authorization guard, but it lives under entities/table-schema/utils. The repo guideline places guards in src/guards/, so moving it there would keep the security layer discoverable and consistent. As per coding guidelines "Place authentication and authorization guards in src/guards/ directory".

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In
`@backend/src/entities/table-schema/utils/schema-change-batch-ownership.guard.ts`
around lines 18 - 19, The SchemaChangeBatchOwnershipGuard class (implements
CanActivate, decorated with `@Injectable`) should be moved from its current
entities/table-schema/utils location into the guards layer (src/guards/);
relocate the file, update any imports that reference
SchemaChangeBatchOwnershipGuard, and update module provider registrations or
exports where it is injected (e.g., any modules that list it in providers or use
it in route guards) so the new path is used; ensure imports for CanActivate and
Injectable remain and run project tests/tsc to catch any remaining import paths
to update.

backend/test/ava-tests/non-saas-tests/non-saas-table-schema-ibmdb2-e2e.test.ts (1)

92-118: Replace any in the DB2 helpers.

params: any and T = any erase the contract for the connection config and query rows right where these tests rely on DB2-specific fields such as schema. A small DB2 connection interface plus a generic queryDb2<T> without the any default keeps these helpers checked and easier to maintain. As per coding guidelines "Always add type annotations to function parameters and return types in TypeScript" and "Avoid any types - use specific types or generics instead."

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In
`@backend/test/ava-tests/non-saas-tests/non-saas-table-schema-ibmdb2-e2e.test.ts`
around lines 92 - 118, Define a specific DB2 params interface (e.g. Db2Params
with database, host, username, password, port, schema: string) and use it
instead of any for buildConnStr(params: Db2Params), ensureSchema(params:
Db2Params) and queryDb2<T>(params: Db2Params, sql: string): Promise<T[]>; remove
the T = any default so callers must supply a concrete row type; update
references to params.schema accordingly and ensure all three
functions/signatures use the new Db2Params type and the precise return type
Promise<T[]> for queryDb2.

backend/test/ava-tests/non-saas-tests/non-saas-table-schema-mongodb-e2e.test.ts (1)

87-91: Missing beforeEach hook to reset nextProposal.

Similar to the MSSQL test file, this MongoDB test suite doesn't reset nextProposal between tests, which could lead to test pollution.

🧹 Proposed fix to add beforeEach reset

 const createdCollections: string[] = [];

+test.beforeEach(() => {
+	nextProposal = null;
+});
+
 function buildMongoUri(params: any): string {

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In
`@backend/test/ava-tests/non-saas-tests/non-saas-table-schema-mongodb-e2e.test.ts`
around lines 87 - 91, Add a beforeEach hook that resets the MockFactory's
nextProposal counter to its initial value to avoid test pollution; specifically,
add beforeEach(() => { mockFactory.nextProposal = 1; }); (or the same reset used
in the MSSQL suite) near where mockFactory is declared so
mockFactory.nextProposal is reset before each test run.

backend/test/ava-tests/non-saas-tests/non-saas-table-schema-oracle-e2e.test.ts (2)

89-92: Missing beforeEach hook to reset nextProposal.

Consistent with the other test files, consider adding a beforeEach hook to reset nextProposal to prevent test pollution.

🧹 Proposed fix to add beforeEach reset

 const testTables: Array<string> = [];

+test.beforeEach(() => {
+	nextProposal = null;
+});
+
 test.before(async () => {

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In
`@backend/test/ava-tests/non-saas-tests/non-saas-table-schema-oracle-e2e.test.ts`
around lines 89 - 92, Tests are missing a beforeEach hook to reset the shared
nextProposal state, which can leak between tests; add a beforeEach that sets
nextProposal = 0 (or its initial value) before each test run to mirror other
test files — locate the tests in non-saas-table-schema-oracle-e2e.test.ts and
add the hook near the top where mockFactory, app, _testUtils, and testTables are
defined so nextProposal is reset prior to each it/test.

---

1-285: Consider expanding Oracle test coverage.

The Oracle test suite has notably fewer tests compared to MySQL and MSSQL. Missing coverage includes:

• Destructive operations requiring confirmedDestructive
• Invalid SQL handling and FAILED status
• userModifiedSql validation
• Index and foreign key operations
• Rollback state constraints

This may be intentional if Oracle is a lower-priority backend, but consider adding parity tests for critical paths.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In
`@backend/test/ava-tests/non-saas-tests/non-saas-table-schema-oracle-e2e.test.ts`
around lines 1 - 285, Add parity tests for Oracle by creating new serial tests
(using createConnection, oracleKnex, randomOracleTableName and nextProposal)
that mirror the MySQL/MSSQL coverage: 1) a destructive change test that sends a
proposal with destructive SQL and asserts the API requires confirmedDestructive
before approve, 2) an invalid-SQL test that returns FAILED status when the
forwardSql is invalid, 3) a userModifiedSql validation test that posts a change
with invalid userModifiedSql and asserts validation error, 4) index and
foreign-key operation tests (CREATE/DROP INDEX, ADD/DROP CONSTRAINT) that verify
the schema changes apply/rollback, and 5) rollback state constraint tests that
attempt rollback when not allowed and assert proper error/status; for each test
use the same endpoints (/table-schema/{connectionId}/generate,
/table-schema/change/{id}/approve, /rollback) and oracle helper
oracleKnex()/oracleColumnExists() to verify DB state.

backend/test/ava-tests/non-saas-tests/non-saas-table-schema-mssql-e2e.test.ts (1)

89-92: Missing beforeEach hook to reset nextProposal.

Unlike the MySQL test file which resets nextProposal in test.beforeEach, this file doesn't reset the test state between tests. If a test fails to set nextProposal, it could inadvertently use a stale value from a previous test, leading to confusing failures.

🧹 Proposed fix to add beforeEach reset

 const testTables: Array<string> = [];

+test.beforeEach(() => {
+	nextProposal = null;
+});
+
 test.before(async () => {

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In
`@backend/test/ava-tests/non-saas-tests/non-saas-table-schema-mssql-e2e.test.ts`
around lines 89 - 92, Add a test.beforeEach hook to reset the shared
nextProposal state before each test to avoid cross-test contamination; locate
the test suite setup near the MockFactory/app/_testUtils/testTables declarations
(symbols: mockFactory, app, _testUtils, testTables) and add a beforeEach that
sets nextProposal back to a neutral value (e.g., undefined or null) so each test
starts with a clean slate.

backend/test/ava-tests/non-saas-tests/non-saas-table-schema-dynamodb-e2e.test.ts (1)

541-578: Missing cleanup registration for test tables.

The test at line 541 (userModifiedSql with mismatched tableName is rejected) does not add tableName to createdTables. If the test fails unexpectedly or the table gets created due to a bug, it won't be cleaned up. The same issue exists in tests starting at lines 580 and 607.

🧹 Proposed fix to ensure cleanup

 test.serial('DynamoDB: userModifiedSql with mismatched tableName is rejected', async (t) => {
 	const { token } = await registerUserAndReturnUserInfo(app);
 	const connectionId = await createConnection(token);
 	const tableName = randomTableName('ra_ddb_umbad');
+	createdTables.push(tableName);
 
 	nextProposal = {

Apply similar fixes for tests at lines 580 and 607.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In
`@backend/test/ava-tests/non-saas-tests/non-saas-table-schema-dynamodb-e2e.test.ts`
around lines 541 - 578, The test "DynamoDB: userModifiedSql with mismatched
tableName is rejected" fails to register the generated table for cleanup—ensure
you push the generated tableName into the shared createdTables array right after
you generate tableName (the variable created by randomTableName) so any
accidental table creation is torn down; locate the test function using symbols
registerUserAndReturnUserInfo, createConnection, randomTableName and
nextProposal and add createdTables.push(tableName). Apply the same change to the
other two tests referenced (the ones that follow, using the same pattern) so
each test that defines a tableName adds it to createdTables for cleanup.

backend/src/entities/table-schema/utils/execute-schema-change.ts (1)

34-43: Inconsistent dispatch: MongoDB only checks changeType, unlike DynamoDB/Elasticsearch which check both.

The DynamoDB (Line 45) and Elasticsearch (Line 56) branches check changeType OR connectionType, but MongoDB only checks changeType. If a MongoDB connection receives a non-Mongo changeType (e.g., due to miscategorization), it would incorrectly fall through to the SQL path and fail.

Consider adding a isMongoDialect(connectionType) check for consistency:

Proposed fix

-	if (isMongoSchemaChangeType(changeType)) {
+	if (isMongoSchemaChangeType(changeType) || isMongoDialect(connectionType)) {
 		const op = validateProposedMongoOp({

Note: You'll need to import isMongoDialect from ./assert-dialect-supported.js.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@backend/src/entities/table-schema/utils/execute-schema-change.ts` around
lines 34 - 43, The Mongo branch currently dispatches solely on
isMongoSchemaChangeType which can let non-Mongo connections be treated as Mongo;
update the dispatch to also verify the connection dialect by checking
isMongoDialect(connectionType) (importing isMongoDialect from
./assert-dialect-supported.js) so the block becomes guarded by both
isMongoSchemaChangeType(changeType) && isMongoDialect(connectionType); keep the
same internals (validateProposedMongoOp, executeMongoSchemaOp) and return
behavior unchanged.

backend/src/entities/table-schema/use-cases/rollback-schema-change.use-case.ts (1)

104-104: Non-null assertion on updated — handle potential null return.

updateStatus can return null per the interface. While unlikely after a successful findByIdWithRelations, defensive handling is safer.

Proposed fix

-		return mapSchemaChangeToResponseDto(updated!);
+		if (!updated) {
+			throw new NotFoundException('Schema change not found after status update.');
+		}
+		return mapSchemaChangeToResponseDto(updated);

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In
`@backend/src/entities/table-schema/use-cases/rollback-schema-change.use-case.ts`
at line 104, The code currently uses a non-null assertion on updated when
returning mapSchemaChangeToResponseDto(updated!), but updateStatus can legally
return null; modify the rollback flow in rollbackSchemaChangeUseCase (after
calling updateStatus) to defensively check for a null return from updateStatus
(variable updated) and handle it — e.g., throw a descriptive error or return a
404/appropriate error response — before calling mapSchemaChangeToResponseDto;
reference the updateStatus call site, the updated variable, the
mapSchemaChangeToResponseDto function, and the earlier findByIdWithRelations
call to ensure consistent error handling.

backend/src/entities/table-schema/utils/validate-proposed-ddl.ts (1)

86-89: Potential false positive: semicolon check doesn't account for string literals or comments.

The check if (/;/.test(stripped)) may incorrectly reject valid single-statement DDL containing semicolons inside string literals (e.g., DEFAULT ';') or comments. Consider this an edge case to monitor — the defense-in-depth approach with parser fallback mitigates risk.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@backend/src/entities/table-schema/utils/validate-proposed-ddl.ts` around
lines 86 - 89, The current multi-statement detection using stripped =
trimmed.replace(/;+\s*$/, '') and if (/;/.test(stripped)) can false-positive on
semicolons inside string literals or comments; update the check in
validate-proposed-ddl (validate-proposed-ddl.ts) to ignore semicolons that occur
inside SQL string literals or comments before deciding it's multi-statement —
implement a lightweight tokenizer that walks trimmed and strips/ignores
single-quoted, double-quoted, dollar-quoted strings and SQL comments (-- ... and
/* ... */), then test the remaining text for semicolons, or reuse the existing
parser fallback path if tokenization fails; ensure you still trim only trailing
statement terminators before this tokenization to preserve legitimate trailing
semicolon handling.

backend/src/entities/table-schema/utils/mongo-schema-op.ts (1)

232-261: Add exhaustive check for unhandled MongoDB operation kinds.

Consistent with the other schema operation utilities, adding a default case improves type safety.

🛡️ Proposed fix

 async function dispatchMongoOp(db: Db, op: MongoSchemaOp): Promise<void> {
 	switch (op.operation) {
 		case 'createCollection': {
 			await db.createCollection(op.collectionName);
 			return;
 		}
 		case 'dropCollection': {
 			await db.collection(op.collectionName).drop();
 			return;
 		}
 		case 'setValidator': {
 			const command: Document = { collMod: op.collectionName };
 			command.validator = op.validatorSchema ?? {};
 			command.validationLevel = op.validationLevel ?? 'strict';
 			command.validationAction = op.validationAction ?? 'error';
 			await db.command(command);
 			return;
 		}
 		case 'createIndex': {
 			if (!op.indexSpec) throw new BadRequestException('createIndex is missing indexSpec.');
 			await db.collection(op.collectionName).createIndex(op.indexSpec, op.indexOptions ?? {});
 			return;
 		}
 		case 'dropIndex': {
 			if (!op.indexName) throw new BadRequestException('dropIndex is missing indexName.');
 			await db.collection(op.collectionName).dropIndex(op.indexName);
 			return;
 		}
+		default: {
+			const _exhaustive: never = op.operation;
+			throw new Error(`Unhandled MongoDB operation: ${_exhaustive}`);
+		}
 	}
 }

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@backend/src/entities/table-schema/utils/mongo-schema-op.ts` around lines 232
- 261, The switch in dispatchMongoOp handling MongoSchemaOp is missing a default
branch for unhandled operation kinds; add an exhaustive default case at the end
of the switch in dispatchMongoOp that throws a clear error (e.g.,
BadRequestException or a similar runtime exception) including the unexpected
op.operation value and possibly the op object for debugging to ensure any
new/unknown MongoSchemaOp variants surface as failures rather than silently
ignored.

backend/src/entities/table-schema/utils/elasticsearch-schema-op.ts (1)

175-201: Add exhaustive check for unhandled operation kinds.

The dispatchElasticsearchOp function's switch statement lacks a default case. While validateProposedElasticsearchOp ensures only valid operations reach this point, adding an exhaustive check improves type safety and catches potential future additions.

🛡️ Proposed fix

 async function dispatchElasticsearchOp(client: Client, op: ElasticsearchSchemaOp): Promise<void> {
 	switch (op.operation) {
 		case 'createIndex': {
 			await client.indices.create({
 				index: op.indexName,
 				body: {
 					...(op.mappings ? { mappings: op.mappings } : {}),
 					...(op.settings ? { settings: op.settings } : {}),
 				},
 			} as Parameters<typeof client.indices.create>[0]);
 			return;
 		}
 		case 'deleteIndex': {
 			await client.indices.delete({ index: op.indexName });
 			return;
 		}
 		case 'updateMapping': {
 			if (!op.properties) {
 				throw new BadRequestException('updateMapping is missing properties.');
 			}
 			await client.indices.putMapping({
 				index: op.indexName,
 				body: { properties: op.properties },
 			} as Parameters<typeof client.indices.putMapping>[0]);
 			return;
 		}
+		default: {
+			const _exhaustive: never = op.operation;
+			throw new Error(`Unhandled Elasticsearch operation: ${_exhaustive}`);
+		}
 	}
 }

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@backend/src/entities/table-schema/utils/elasticsearch-schema-op.ts` around
lines 175 - 201, The switch in dispatchElasticsearchOp does not handle unknown
op.operation kinds—add an exhaustive default branch that throws a runtime error
(or uses a utility like assertUnreachable) when encountering an unexpected
operation to make the function fail-fast and preserve type safety; update
dispatchElasticsearchOp to include a default that references the incoming
op.operation in the thrown message and mention validateProposedElasticsearchOp
in a comment if desired so future changes to operation kinds are caught here.

backend/src/entities/table-schema/table-schema.controller.ts (1)

42-47: Remove redundant @Injectable() decorator from controller.

Controllers in NestJS are automatically injectable. The @Injectable() decorator is redundant when used with @Controller() and may cause confusion about the class's role.

♻️ Proposed fix

 `@UseInterceptors`(SentryInterceptor)
 `@Controller`()
 `@ApiBearerAuth`()
 `@ApiTags`('Table Schema Changes')
-@Injectable()
 export class TableSchemaController {

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@backend/src/entities/table-schema/table-schema.controller.ts` around lines 42
- 47, Remove the redundant `@Injectable`() decorator from the
TableSchemaController class: locate the class declaration for
TableSchemaController that is annotated with `@Controller`(),
`@UseInterceptors`(SentryInterceptor), `@ApiBearerAuth`(), and `@ApiTags`('Table
Schema Changes') and delete the `@Injectable`() line so the controller relies on
Nest's automatic injectability provided by `@Controller`().

backend/src/entities/table-schema/utils/dynamodb-schema-op.ts (1)

222-278: Add exhaustive check for unhandled DynamoDB operation kinds.

Similar to the Elasticsearch utility, the switch statement lacks a default case for type exhaustiveness.

🛡️ Proposed fix

 async function dispatchDynamoDbOp(client: DynamoDB, op: DynamoDbSchemaOp): Promise<void> {
 	switch (op.operation) {
 		case 'createTable': {
 			// ... existing code ...
 			return;
 		}
 		case 'deleteTable': {
 			// ... existing code ...
 			return;
 		}
 		case 'updateTable': {
 			// ... existing code ...
 			return;
 		}
 		case 'updateTimeToLive': {
 			// ... existing code ...
 			return;
 		}
+		default: {
+			const _exhaustive: never = op.operation;
+			throw new Error(`Unhandled DynamoDB operation: ${_exhaustive}`);
+		}
 	}
 }

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@backend/src/entities/table-schema/utils/dynamodb-schema-op.ts` around lines
222 - 278, The switch in dispatchDynamoDbOp (handling DynamoDbSchemaOp) is
missing an exhaustive/default case; add a final default branch (or an
assertUnreachable) that throws a clear error (e.g., BadRequestException or
Error) when op.operation is an unknown/unsupported value so unhandled operation
kinds fail loudly and the type switch is exhaustive.

backend/src/entities/table-schema/use-cases/approve-and-apply-schema-change.use-case.ts (1)

131-136: Consider using optimistic locking for APPLYING → APPLIED transition.

The updateStatus call at line 131-135 doesn't verify the expected prior status (APPLYING). If a concurrent process somehow modified the status (e.g., a timeout mechanism marking it FAILED), this could overwrite that state. The code already uses transitionStatusIfMatches at line 114 for the initial transition; consider using it here for consistency.

🔒 Proposed fix

-			const updated = await this._dbContext.tableSchemaChangeRepository.updateStatus(
+			const updated = await this._dbContext.tableSchemaChangeRepository.transitionStatusIfMatches(
 				change.id,
-				SchemaChangeStatusEnum.APPLIED,
-				{ appliedAt: new Date(), executionError: null },
+				SchemaChangeStatusEnum.APPLYING,
+				SchemaChangeStatusEnum.APPLIED,
+				{ appliedAt: new Date(), executionError: null }
 			);
+			if (!updated) {
+				this.logger.warn(`Status changed concurrently after successful execution for change ${change.id}`);
+			}
 			return mapSchemaChangeToResponseDto(updated!);

Note: This depends on whether transitionStatusIfMatches supports additional fields. If not, a two-step approach (transition then update fields) may be needed.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In
`@backend/src/entities/table-schema/use-cases/approve-and-apply-schema-change.use-case.ts`
around lines 131 - 136, The updateStatus call on tableSchemaChangeRepository
that sets SchemaChangeStatusEnum.APPLIED (used inside
approve-and-apply-schema-change.use-case.ts) lacks an optimistic lock check for
the prior APPLYING state and can overwrite concurrent state changes; replace
this updateStatus(...) call with a call to transitionStatusIfMatches(change.id,
SchemaChangeStatusEnum.APPLYING, SchemaChangeStatusEnum.APPLIED, { appliedAt:
new Date(), executionError: null }) if transitionStatusIfMatches supports extra
fields, or if not, perform a two-step safe sequence: first call
transitionStatusIfMatches(change.id, SchemaChangeStatusEnum.APPLYING,
SchemaChangeStatusEnum.APPLIED) and only when that returns success call the
existing updateStatus to set appliedAt/executionError, then return
mapSchemaChangeToResponseDto(updated) as before.

backend/src/guards/schema-change-batch-ownership.guard.ts (1)

35-47: ⚠️ Potential issue | 🟠 Major

Authorize the whole batch, not an arbitrary member row.

findOne({ where: { batchId } }) checks permissions against whichever batch row happens to come back first. If a batch ever contains changes from multiple connectionIds, a user with access to one connection could operate on changes from another. Load the distinct connectionIds for the batch, require exactly one, then authorize against that value.

Suggested fix

-		const member = await this.tableSchemaChangeRepository.findOne({
-			where: { batchId },
-			select: ['id', 'connectionId'],
-		});
-		if (!member) {
+		const batchConnections = await this.tableSchemaChangeRepository
+			.createQueryBuilder('schema_change')
+			.select('DISTINCT schema_change.connectionId', 'connectionId')
+			.where('schema_change.batchId = :batchId', { batchId })
+			.getRawMany<{ connectionId: string }>();
+		if (batchConnections.length === 0) {
 			throw new NotFoundException('Schema change batch not found.');
 		}
+		if (batchConnections.length > 1) {
+			throw new BadRequestException('Schema change batch spans multiple connections.');
+		}

 		const allowed = await this.cedarAuthService.validate({
 			userId,
 			action: CedarAction.ConnectionEdit,
-			connectionId: member.connectionId,
+			connectionId: batchConnections[0].connectionId,
 		});

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@backend/src/guards/schema-change-batch-ownership.guard.ts` around lines 35 -
47, The guard currently authorizes using a single arbitrary row returned by
tableSchemaChangeRepository.findOne which can pick a connectionId from any
member; change it to load all distinct connectionId values for the given batchId
(e.g., query tableSchemaChangeRepository for connectionId where batchId and
dedupe) require that the resulting set has exactly one distinct connectionId
(throw NotFoundException or a specific error if none, and a 400/Forbidden if
multiple), and then call cedarAuthService.validate with that single connectionId
and CedarAction.ConnectionEdit instead of using member.connectionId from
findOne; update variable names (batchConnectionIds or distinctConnectionIds) and
remove reliance on findOne to ensure the whole batch is authorized against a
single connection.

docker-compose.tst.yml (1)

269-273: Use env substitution instead of inline basic-auth credentials in healthcheck.

Line 272 currently hardcodes elastic:SuperSecretElasticPassword in the command string. Prefer the service env var to reduce credential exposure in metadata and logs.

🔧 Suggested change

-          "curl -s -u elastic:SuperSecretElasticPassword http://localhost:9200/_cluster/health | grep -E '\"status\":\"(green|yellow)\"'",
+          "curl -fsS -u elastic:$$ELASTIC_PASSWORD http://localhost:9200/_cluster/health | grep -E '\"status\":\"(green|yellow)\"'",

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@docker-compose.tst.yml` around lines 269 - 273, The healthcheck command under
the test service currently hardcodes basic-auth credentials; change it to use
environment substitution (e.g., ${ELASTIC_USERNAME} and ${ELASTIC_PASSWORD} or a
single ${ELASTIC_CREDENTIALS}) so the curl -u value is not inline. Update the
test service's env block to export those variables and replace the inline
"elastic:SuperSecretElasticPassword" in the CMD-SHELL string with the
interpolated vars (keeping the CMD-SHELL form and proper quoting) so credentials
come from service environment rather than appearing directly in the healthcheck
command.

docker-compose.yml (1)

179-183: Avoid embedding credentials directly in the healthcheck command string.

Line 182 includes inline basic auth credentials. Even in test stacks, this leaks into container metadata/process args and tends to trip secret scanners. Prefer using the existing env var in the command.

🔧 Suggested change

-          "curl -s -u elastic:SuperSecretElasticPassword http://localhost:9200/_cluster/health | grep -E '\"status\":\"(green|yellow)\"'",
+          "curl -fsS -u elastic:$$ELASTIC_PASSWORD http://localhost:9200/_cluster/health | grep -E '\"status\":\"(green|yellow)\"'",

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@docker-compose.yml` around lines 179 - 183, The healthcheck 'test' command
currently embeds the basic auth credentials inline; update the healthcheck under
the 'test' entry to use the existing environment variable (e.g.,
ELASTIC_PASSWORD) instead of hardcoding the password by invoking curl with -u
"elastic:$ELASTIC_PASSWORD" (wrap the curl in a shell invocation like sh -c or
the equivalent array form so the container runtime expands the env var) so
credentials come from the environment rather than appearing in the command
string or process args.

backend/src/guards/schema-change-ownership.guard.ts (1)

18-52: Consider sharing the schema-change authorization flow.

This guard and backend/src/guards/schema-change-batch-ownership.guard.ts now duplicate request extraction, UUID validation, repository lookup, and Cedar authorization. A small shared helper/service would make future permission fixes land in one place and keep the error semantics aligned.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@backend/src/guards/schema-change-ownership.guard.ts` around lines 18 - 52,
Both SchemaChangeOwnershipGuard and schema-change-batch-ownership.guard
duplicate request extraction, UUID validation, repository lookup, and Cedar
authorization; extract that logic into a shared service (e.g.,
SchemaChangeAuthorizationService) with a method like
validateChangeOwnership(userId: string, changeId: string) that: validates the
changeId (reusing ValidationHelper.isValidUUID), loads the
TableSchemaChangeEntity (selecting id and connectionId) from the repository,
calls cedarAuthService.validate({ userId, action: CedarAction.ConnectionEdit,
connectionId }), and throws the same
BadRequestException/NotFoundException/ForbiddenException as the guards currently
do; update both guards' canActivate methods to call this new service method and
return true on success.